Skip to content

build: migrate from Gradle to the Kotlin Toolchain#49

Open
singleton11 wants to merge 9 commits into
sdkman:mainfrom
singleton11:migrate-to-kotlin-toolchain
Open

build: migrate from Gradle to the Kotlin Toolchain#49
singleton11 wants to merge 9 commits into
sdkman:mainfrom
singleton11:migrate-to-kotlin-toolchain

Conversation

@singleton11
Copy link
Copy Markdown

@singleton11 singleton11 commented May 27, 2026

Summary

Replaces the Gradle build with the Kotlin Toolchain.

What's preserved

  • gradle/libs.versions.toml — single source of truth for all versions, reused as-is by the Toolchain. Every coordinate in module.yaml, plugins/*/module.yaml, and plugins/*/plugin.yaml is a $libs.* reference.
  • detekt.yml, .editorconfig, docker-compose.yml, all broker source under src/ — unchanged.
  • MetaReleaseService and its tests — byte-identical to main (verified via git diff main -- src/main/kotlin/.../MetaReleaseService.kt src/test/...). The release plugin's writeReleaseProperties task action publishes the legacy release.properties format so the existing Properties.load() consumer keeps working with zero source churn.

What replaces what

Old New
./gradlew build ./kotlin build
./gradlew test ./kotlin test
./gradlew check ./kotlin check (runs detekt + ktlint via their checks: registrations + tests)
./gradlew run ./kotlin run
./gradlew currentVersion ./kotlin do currentVersion
./gradlew release ./kotlin do release
./gradlew jib ./kotlin do jib (or ./kotlin do jibDockerBuild / jibBuildTar)
./gradlew ktlintFormat ./kotlin do ktlintFormat
generateReleaseProperties task release plugin's writeReleaseProperties task (auto-wired into generated.resources)

@singleton11 singleton11 force-pushed the migrate-to-kotlin-toolchain branch from 80d1f0e to 04522db Compare May 27, 2026 16:19
Co-authored-by: Cursor <cursoragent@cursor.com>
@singleton11 singleton11 force-pushed the migrate-to-kotlin-toolchain branch from 04522db to e89f243 Compare May 27, 2026 16:32
@marc0der
Copy link
Copy Markdown
Member

@claude please review this PR.

@chloe41427
Copy link
Copy Markdown
Contributor

chloe41427 Bot commented May 28, 2026

Review: migrate from Gradle to the Kotlin Toolchain

Solid, well-scoped migration. Broker source under src/ is genuinely byte-identical to main (confirmed via the PR file list, zero src/ entries), the version catalog is reused as-is, and the build/CI changes line up cleanly with the new wrapper. CI's build job on the PR HEAD is green. Reviewed by reading the full 3.3k-line diff plus the existing MetaReleaseService.kt it has to stay compatible with.

What's done well

  • gradle/libs.versions.toml preserved as the catalog, and module.yaml / plugin yamls only reference it via $libs.*. Single source of truth for versions stays intact.
  • build.gradle.kts stub kept for Dependabot detection, with a self-documenting comment explaining exactly why. The grouping rules in .github/dependabot.yml are updated to cover the new local-plugin runtime deps (detekt-cli, ktlint-cli, jib-core, jgit, bouncycastle, slf4j).
  • release.properties compatibility is handled deliberately: the writeReleaseProperties task in plugins/release/src/tasks/WriteReleaseProperties.kt writes the legacy single-line release=<version> shape that MetaReleaseService already loads via Properties.load(). Verified by reading MetaReleaseServiceImpl: it reads the resource named release.properties from the classloader root, and plugin.yaml wires the output dir into generated.resources. Zero source churn in the consumer, as advertised.
  • Release plugin port is a thoughtful axion-release shrinkwrap: currentVersion, verifyRelease, createRelease, pushRelease, release, with RELEASE_FORCE_VERSION / RELEASE_DISABLE_CHECKS escape hatches and a sane nearestTagFromHead walk via JGit. Concurrency guard in release.yml preserved.
  • BouncyCastle pin is documented in-place (gradle/libs.versions.toml and plugins/release/module.yaml) with the actual reason (Apache MINA SSHD optional-at-compile, hard-required-at-runtime). Future-me will thank you.
  • KOTLIN_CLI_NO_WELCOME_BANNER set on both workflows. Keeps the CI log clean.

Substantive concern: branch decoration leaks into release-time currentVersion

plugins/release/src/version/Pipeline.kt::infer() always runs decorate(), even when HEAD is exactly on the release tag:

val onTag = nearest.onHead && !forceSnapshot
val effective = if (onTag) parsed else parsed.incrementPatch()
val decorated = decorate(effective.toString(), repo)        // unconditional
val withSnapshot = if (onTag) decorated else appendSnapshot(decorated)

decorate() returns "$version-${sanitize(branch)}" for any branch that is not exactly main or master (and not blank). GitRepo.branchName() delegates to JGit's Repository.getBranch(), which on detached HEAD returns the abbreviated commit SHA, not the branch name. actions/checkout@v6 checks out commits in detached mode by default for push events.

release.yml runs in exactly this state:

./kotlin do release            # creates v1.2.4 at HEAD via nextReleaseVersion(); bare semver, OK
current_version=$(./kotlin do currentVersion 2>/dev/null | tail -n1)
./kotlin do jib --setting "jib.targetImage.tags=[$current_version,$commit_hash,latest]"

After release tags HEAD with v1.2.4, the subsequent currentVersion call enters the NearestTag.Found branch with onTag = true, then decorate("1.2.4", repo) sees branch == "<abbrev SHA>" (not main/master, not blank) and returns "1.2.4-abcdef1". That string then propagates into the Jib targetImage tags. Old axion behavior with versionSeparator = "" and no versionWithBranch produced bare 1.2.4, so this is a behavioral regression on the release path that the PR description doesn't call out.

Two reasonable fixes; either is fine, both is better:

  1. In Pipeline.infer(), skip decorate() when onTag is true. This matches the spirit of axion's versionWithBranch (decoration is for snapshot/dev builds, not for released versions).
  2. In Pipeline.decorate() (or in effectiveBranchName), treat detached-HEAD as the main-branch case. GitRepo.isDetached() is already there; use it. Optionally set RELEASE_OVERRIDDEN_BRANCH_NAME=main in release.yml as a belt-and-braces hedge.

Worth verifying with gh run view <release-run> after merging by sanity-checking the image tags on DOCR vs the git tag.

Smaller things worth tidying

  • com.example.release package is a placeholder name across plugins/release/src/** and plugins/release/plugin.yaml. Should be io.sdkman.amper.plugins.release (matching the ktlint and package plugins). com.example.* will read as scaffolding leftover six months from now.
  • Jib plugin uses the default (unnamed) package in plugins/jib/src/Jib.kt and settings.kt. The settingsClass is referenced unqualified as JibSettings in module.yaml. Works, but mixing default-package with explicitly-packaged plugins is inconsistent; give it io.sdkman.amper.plugins.jib while you're at it.
  • plugins/package/src/Settings.kt::artifactName is declared with a docstring (Optional override for the file name of the staged JAR) but never referenced in plugins/package/plugin.yaml, which hard-codes outputJar: ${module.rootDir}/build/libs/${module.name}.jar. Either wire it (${pluginSettings.artifactName} with a fallback) or drop the setting until it's needed.
  • No tests on the release plugin. SemVer parsing, nearestTagFromHead, snapshot/decoration logic, pickTagToPush (env precedence, multi-tag-at-HEAD ordering); none of it covered. This is non-trivial behavior that axion's well-tested implementation used to cover for you. The release-only workflow path means a regression won't be caught until prod cuts a tag. At minimum, unit tests for SemVer.parseOrNull, VersionPipeline.infer() over a fake repo, and pickTagToPush would be cheap and high-value.
  • ./kotlin do currentVersion 2>/dev/null | tail -n1 discards stderr (kills useful failure diagnostics) and assumes the last stdout line is the version. The task does println(inferred.version) as its only intentional stdout, but the launcher may print task-status banners. Please verify locally that tail -n1 resolves to the version under both quiet and verbose toolchain modes, and consider a --quiet flag if one exists. Losing stderr also means a JGit auth failure becomes "current version is empty" instead of a readable message; would capturing to a file and grepping be more robust?
  • plugins/detekt/src/CommonRunDetectSettings.kt is missing a trailing newline (the \ No newline at end of file marker in the diff). Detekt itself wouldn't care; ktlint might.
  • Detekt-plugin copyright/license attribution is preserved (Copyright 2000-2025 JetBrains s.r.o. ... Apache 2.0); good. Worth confirming the ktlint and release plugin sources don't owe similar attribution to any code they were adapted from. The release plugin is described as a port of axion, and axion is Apache 2.0, so a one-line attribution comment at the top of plugins/release/src/version/Pipeline.kt would be a courtesy.
  • CLAUDE.md still says ./kotlin clean exists. Does it? The plugin yamls in this PR don't register a clean command; toolchain may provide it natively, but worth a sentence in the docs about that distinction.

Compatibility / risk summary

Area Behavior on main Behavior on this PR Verdict
MetaReleaseService runtime load of release.properties works works (writeReleaseProperties stages it in the resources root) unchanged
CI artifact upload at build/libs/ yes (Gradle application) yes (package plugin) unchanged
Released Docker image tags [<version>, <commit>, latest] [1.2.4, abc, latest] [1.2.4-<sha>, abc, latest] on detached HEAD regression, see concern above
Tag created at HEAD v1.2.4 (axion) v1.2.4 (release plugin nextReleaseVersion bypasses decorate) unchanged
Detekt + ktlint enforcement on ./kotlin check yes yes unchanged
Jib base image / ports / env / user yes yes (module.yaml mirrors faithfully) unchanged

The bulk of this PR is good and lands an ambitious migration cleanly. The branch-decoration-on-detached-HEAD issue is the one thing I'd want fixed before merging, since it would silently change the published Docker image tag scheme on the first post-merge release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants